Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC] Add documentation for database downloader setting #210

Conversation

edmocosta
Copy link
Contributor

Related issue: #14823

@edmocosta edmocosta marked this pull request as ready for review December 27, 2022 15:45
Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edmocosta, I left some comments for your consideration. You can decide. :-)
Thanks for adding this helpful info!

docs/index.asciidoc Outdated Show resolved Hide resolved
docs/index.asciidoc Outdated Show resolved Hide resolved
@edmocosta
Copy link
Contributor Author

Hi @karenzone! Thank you very much for reviewing!
I've just updated the docs to reflect recent changes discussed on the #14823 PR. Could you please take a look at it again?

@edmocosta edmocosta changed the title [DOC] Add documentation for database auto-update configuration [DOC] Add documentation for database downloader setting Jan 5, 2023
Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments for consideration. Let me know if you'd like to discuss.

docs/index.asciidoc Outdated Show resolved Hide resolved
docs/index.asciidoc Outdated Show resolved Hide resolved
docs/index.asciidoc Outdated Show resolved Hide resolved
Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My github session expired mid-review and blew away some comments/explanations. I hope that no context is lost.

I left some notes about being more clear that the auto-update and database settings are actually set in two different places--the plugin and logstash.yml.

docs/index.asciidoc Outdated Show resolved Hide resolved
docs/index.asciidoc Outdated Show resolved Hide resolved
Co-authored-by: Karen Metts <35154725+karenzone@users.noreply.github.com>
Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builds cleanly, renders as expected, and LGTM! Thanks for adding this feature and clarifying doc.

@edmocosta edmocosta merged commit fc2df01 into logstash-plugins:main Feb 7, 2023
@edmocosta edmocosta deleted the add-geoip-db-auto-update-setting-docs branch February 7, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants